Repository navigation
Conversation
…ectory In a repo like apps/api/app/core/db.py imported as app.core.db, the module graph had no edges and every module was listed as isolated. resolveImport only matches when the module path is the shorter side, but here the import's leading part is the tail of the module path. Add a Python case to langResolver, modelled on the Java one. It indexes each .py file under every trailing sub-path of its directory and of its own path, resolves a dotted import by its longest matching leading part, and picks the closest module when several match. A single segment import only matches a package (a directory with an __init__.py), so import logging does not resolve to a repo file called logging.py. Relative imports and anything unmatched still go to the generic resolver, so this only adds edges. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66003dcea1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case ".py": | ||
| r.notePython(p, dir, mod, pySeen) |
There was a problem hiding this comment.
Exclude skipped modules from the Python candidate index
When a scan contains pkg/__init__.py, app/main.py importing pkg, and app/tests/pkg/__init__.py, this registers the excluded test package as a candidate. closestModule selects app/tests/pkg, but Build discards it via shouldSkipModule(toMod) in graph.go:175-176; because resolution was already marked handled, the valid app -> pkg dependency disappears. The parent commit resolves this edge through knownModules, which excludes test modules. Filter skipped modules out before adding Python candidates.
Useful? React with 👍 / 👎.
| if mods := r.pyFiles[key]; n >= 2 && len(mods) > 0 { | ||
| return closestModule(p, mods) |
There was a problem hiding this comment.
Prefer a package over a same-named Python file
If pkg/util.py and pkg/util/__init__.py coexist, an import pkg.util in consumer/main.py returns the file's containing module, pkg, here before considering the pkg/util package candidate. Python imports the package in this layout, and the parent commit's generic resolver also chose the exact pkg/util module. This therefore rewrites the dependency from consumer -> pkg/util to consumer -> pkg, producing an incorrect module graph. Prefer the matching package at the same import root before accepting the file.
Useful? React with 👍 / 👎.
What and why
In a monorepo where the Python package sits below the scanned root (for example
apps/api/app/core/db.py, imported asfrom app.core.db import Session), the module graph has no edges and every module is listed as isolated. The Map tab shows unconnected circles, anddependencies.edgesis empty.resolveImportturnsapp.core.dbintoapp/core/dband matches it to a module only when the import ends with the module name, starts with it, or ends with it (Go style). All three assume the module path is the shorter side. Here the module isapps/api/app/coreand the import's leadingapp/coreis the tail of that path.langResolverhas Java, Ruby and Rust cases but no Python one.This adds a Python case, modelled on the Java one:
.pyfile is indexed under every trailing sub-path of its directory and of its own path without.py;__init__.py), soimport loggingdoes not resolve to a repo file calledlogging.py;How it was checked
go vet ./...andgo test ./... -racepassfrom app.imports and 4 relative ones, laid out asapps/api/app/.... Module edges went from 0 to 418 and isolated modules from 89 to 14, with the same files and config. The symbol graph for the same repo already had 12,711 call edges and 2,266 import edges.The two new tests in
modresolve_test.gofail without the change (emptydepended_by) and pass with it. They cover resolving through a file and through a package, a stdlib name that clashes with a repo file (logging), a third party name that matches a directory without an__init__.py(alembic), and single segment imports.Docs
go run ./benchoutput (no size or speed claims)🤖 Generated with Claude Code